[Woo POS] Coupons: Disallow adding duplicate coupons to cart #15551

iamgabrielma · 2025-04-25T04:04:17Z

Description

Partially addresses WOOMOB-259 by disallowing duplicated coupons to be added to the cart, while we get feedback from design on how to handle it UI-wise.

We also avoid calling the pos_coupon_added_to_cart track event multiple times by creating a new implementation for POSItemActionHandler, otherwise we could see the track event called multiple times despite the action being disallowed, as we used the same action handler across products and coupons:

🔵 Tracked pos_coupon_added_to_cart, properties: [blog_id: -1, is_wpcom_store: false, plan: , was_ecommerce_trial: false, site_url: https://indiemelon.mystagingwebsite.com, store_id: c5bd46cc-1804-4f7b-badb-bb98c449127f]
🔵 Tracked pos_coupon_added_to_cart, properties: [is_wpcom_store: false, plan: , store_id: c5bd46cc-1804-4f7b-badb-bb98c449127f, was_ecommerce_trial: false, blog_id: -1, site_url: https://indiemelon.mystagingwebsite.com]
🔵 Tracked pos_coupon_added_to_cart, properties: [site_url: https://indiemelon.mystagingwebsite.com, store_id: c5bd46cc-1804-4f7b-badb-bb98c449127f, plan: , is_wpcom_store: false, blog_id: -1, was_ecommerce_trial: false]

Testing information

In POS, add a coupon to the cart
Try again, observe how nothing happens. Removing it allows to be re-added.
Confirm the pos_coupon_added_to_cart event is only logged once, when we add the coupon to the cart
Confirm that adding products and variations to cart still work as expected.

Screenshots

Screen.Recording.2025-04-25.at.10.34.12.mov

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on all devices (phone/tablet) and no regressions are added.

We cannot extract the function to make SUT for the whole test suite (at least not straight-away) as crashes due analytics running on different threads that we expect for each enum in the test suite.

Despite disallowing for duplicated coupons to be added to cart, the action handler is generic to all items so we were still calling the tracking event. We create a handler specifically for coupons to avoid calling the event when it shouldnt

wpmobilebot · 2025-04-25T04:16:23Z

📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce iOS Prototype
	Build Number	`29672`
	Version	`PR #15551`
	Bundle ID	`com.automattic.alpha.woocommerce`
	Commit	`d004acc`
	Installation URL	62bvkadr7jv2o

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

staskus · 2025-04-25T08:02:19Z

@iamgabrielma thanks for the work!

Let's wait before merging until we get input from design. I think it's better right now to show an error in the checkout rather than not allow tapping the item without any visual signal.

Also, let's clarify if the item should be tappable when it's disabled (including the expired one). Maybe we wouldn't need a separate item handler logic.

Thanks!

iamgabrielma · 2025-05-02T02:26:21Z

Confirmed with design FnDpCHVQ3Zpefad04ptbd4-fi-node-id=419-79264#1230020091 that nothing should happen when attempting to add a duplicated coupon to the cart. Will resolve the merge conflicts EOD and re-submit 👍

wpmobilebot · 2025-05-02T03:34:40Z

Version 22.3 has now entered code-freeze, so the milestone of this PR has been updated to 22.4.

…-twice # Conflicts: # WooCommerce/Classes/POS/Presentation/ItemListView.swift # WooCommerce/WooCommerceTests/POS/Models/PointOfSaleAggregateModelTests.swift

Not that reusable, as analytics is not thread safe and crashes randomly some tests. For now let’s declare the dependencies per test basis

staskus · 2025-05-05T07:08:24Z

@iamgabrielma to confirm, is it ready for review?

staskus

Thanks! Just remove the unused code 👍

staskus · 2025-05-05T11:19:51Z

WooCommerce/Classes/POS/Presentation/Item Selector/POSItemActionHandler.swift

@@ -51,6 +51,28 @@ final class StandardPOSItemActionHandler: POSItemActionHandler {
    }
 }

+@available(iOS 17.0, *)
+final class CouponsItemActionHandler: POSItemActionHandler {


No longer needed, can be removed 👍

Thanks, I didn't see that we stopped using CouponsItemActionHandler since I worked on this bit. Removing it introduces a regression thought, as we treat products and coupons equally when dealing with track events, meaning: We will track the add_to_cart event despite coupons not being added to cart when are duplicated:

🔵 Tracked pos_coupon_added_to_cart, properties: [site_url: https://indiemelon.mystagingwebsite.com, blog_id: -1, was_ecommerce_trial: false, plan: , store_id: c5bd46cc-1804-4f7b-badb-bb98c449127f, is_wpcom_store: false] 🔵 Tracked pos_coupon_added_to_cart, properties: [store_id: c5bd46cc-1804-4f7b-badb-bb98c449127f, plan: , is_wpcom_store: false, blog_id: -1, was_ecommerce_trial: false, site_url: https://indiemelon.mystagingwebsite.com]

I've added a small helper function to handle this case in POSItemActionHandler, as this will happen both when we add coupons via list and via search.

I'll leave this one without merging for now in case you want to take a look, in a nutshell when adding coupons from either list or search, the track event should be logged just once.

iamgabrielma · 2025-05-06T03:50:25Z

to confirm, is it ready for review?

Yes! Sorry I wasn't clear 🙇

Since the specific CouponsItemActionHandler is not used anymore, we still need to deal with duplicates coupons in the general handler to avoid duplicated track events

staskus · 2025-05-06T06:20:15Z

WooCommerce/Classes/POS/Models/Cart.swift

@@ -49,7 +49,9 @@ extension Cart {
        case .variableParentProduct:
            return
        case .coupon(let coupon):
-            let couponItem = Cart.CouponItem(id: UUID(), code: coupon.code, summary: coupon.summary)
+            guard !coupons.contains(where: { $0.id == coupon.id }) else { return }


Do we need this check here if we skip the duplicate in the action handler?

We don't anymore, I've removed this on 68096bc and added initial tests for the action handler on a7c5977

staskus

👍

iamgabrielma added 3 commits April 25, 2025 10:16

Return early if same coupon ID is already in cart

11913c3

extract sut to be reusable within cart struct

b3a53cb

We cannot extract the function to make SUT for the whole test suite (at least not straight-away) as crashes due analytics running on different threads that we expect for each enum in the test suite.

iamgabrielma added type: task An internally driven task. feature: coupons Related to basic fulfillment such as order tracking. feature: POS labels Apr 25, 2025

iamgabrielma added this to the 22.3 milestone Apr 25, 2025

iamgabrielma marked this pull request as ready for review April 25, 2025 04:05

lint

72e42f7

iamgabrielma requested a review from staskus April 25, 2025 04:06

wpmobilebot modified the milestones: 22.3, 22.4 May 2, 2025

iamgabrielma added 3 commits May 2, 2025 10:40

Merge branch 'trunk' into task/woomob-259-disallow-adding-same-coupon…

6868fe0

…-twice # Conflicts: # WooCommerce/Classes/POS/Presentation/ItemListView.swift # WooCommerce/WooCommerceTests/POS/Models/PointOfSaleAggregateModelTests.swift

Delete unused makeSUT

ff28aea

Not that reusable, as analytics is not thread safe and crashes randomly some tests. For now let’s declare the dependencies per test basis

restore using analytics shared dependency for CartTests

936f2b5

staskus approved these changes May 5, 2025

View reviewed changes

Remove unused CouponsItemActionHandler

3bfe659

Since the specific CouponsItemActionHandler is not used anymore, we still need to deal with duplicates coupons in the general handler to avoid duplicated track events

staskus reviewed May 6, 2025

View reviewed changes

remove early return duplication

68096bc

staskus approved these changes May 6, 2025

View reviewed changes

iamgabrielma added 3 commits May 6, 2025 14:01

Add POSItemActionHandlerTests

3c28116

extract MockPOSSearchHistoryService

a7c5977

use specific handler type when testing search

d004acc

iamgabrielma enabled auto-merge May 6, 2025 07:21

iamgabrielma merged commit 77ca554 into trunk May 6, 2025
13 checks passed

iamgabrielma deleted the task/woomob-259-disallow-adding-same-coupon-twice branch May 6, 2025 07:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Woo POS] Coupons: Disallow adding duplicate coupons to cart #15551

[Woo POS] Coupons: Disallow adding duplicate coupons to cart #15551

Uh oh!

iamgabrielma commented Apr 25, 2025 •

edited

Loading

Uh oh!

wpmobilebot commented Apr 25, 2025 •

edited

Loading

Uh oh!

staskus commented Apr 25, 2025 •

edited

Loading

Uh oh!

iamgabrielma commented May 2, 2025

Uh oh!

wpmobilebot commented May 2, 2025

Uh oh!

staskus commented May 5, 2025

Uh oh!

staskus left a comment

Uh oh!

staskus May 5, 2025

Uh oh!

iamgabrielma May 6, 2025 •

edited

Loading

Uh oh!

iamgabrielma commented May 6, 2025

Uh oh!

staskus May 6, 2025

Uh oh!

iamgabrielma May 6, 2025 •

edited

Loading

Uh oh!

staskus left a comment

Uh oh!

Uh oh!

Uh oh!

[Woo POS] Coupons: Disallow adding duplicate coupons to cart #15551

[Woo POS] Coupons: Disallow adding duplicate coupons to cart #15551

Uh oh!

Conversation

iamgabrielma commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing information

Screenshots

Reviewer (or Author, in the case of optional code reviews):

Uh oh!

wpmobilebot commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

staskus commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iamgabrielma commented May 2, 2025

Uh oh!

wpmobilebot commented May 2, 2025

Uh oh!

staskus commented May 5, 2025

Uh oh!

staskus left a comment

Choose a reason for hiding this comment

Uh oh!

staskus May 5, 2025

Choose a reason for hiding this comment

Uh oh!

iamgabrielma May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iamgabrielma commented May 6, 2025

Uh oh!

staskus May 6, 2025

Choose a reason for hiding this comment

Uh oh!

iamgabrielma May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

staskus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

iamgabrielma commented Apr 25, 2025 •

edited

Loading

wpmobilebot commented Apr 25, 2025 •

edited

Loading

staskus commented Apr 25, 2025 •

edited

Loading

iamgabrielma May 6, 2025 •

edited

Loading

iamgabrielma May 6, 2025 •

edited

Loading